Refactor relational queries - #1131
Merged
Merged
Conversation
2 tasks
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1131 +/- ##
==========================================
+ Coverage 92.28% 92.54% +0.26%
==========================================
Files 51 51
Lines 7804 7757 -47
==========================================
- Hits 7202 7179 -23
+ Misses 602 578 -24
🚀 New features to boost your workflow:
|
…_table - Replace the bespoke numpy implementation in _filter_table_by_elements with a call to join_spatialelement_table(how="left"), removing the unused match_rows parameter and consolidating to a single code path. - Fix join functions (_left_join, _inner_join, _right_exclusive_join) to update spatialdata_attrs region metadata after filtering. - Fix _right_exclusive_join: restore groupby pattern (consistent with other joins) and use reset_index so integer positions are used as the group index, avoiding IndexError when obs names are duplicated and fixing a latent bug where pd.concat of per-group masks produced a partial-length boolean mask. - Simplify _get_filtered_or_unfiltered_tables to use _filter_table_by_elements. - Replace SpatialData.init_from_elements with get_model() lookup when sdata=None in join_spatialelement_table, removing an expensive importlib call. - Add test_join_updates_spatialdata_attrs covering metadata update for all join types; update test_filter_by_table_query_edge_cases to match new behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
LucaMarconato
marked this pull request as ready for review
May 15, 2026 17:35
…by_elements The old function filtered only by region name, ignoring instance IDs. Replace its only caller (filter_tables by coordinate system) with _filter_table_by_elements, which correctly filters by both region and instance. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
LucaMarconato
enabled auto-merge (squash)
May 15, 2026 17:39
LucaMarconato
disabled auto-merge
May 15, 2026 17:54
Tomatokeftes
added a commit
to Tomatokeftes/spatialdata
that referenced
this pull request
Jun 3, 2026
…scverse#1131) Upstream PR scverse#1131 refactored _filter_table_by_elements to delegate to join_spatialelement_table(how="left"), removed _filter_table_by_element_names, and dropped the unused match_rows parameter. The lazy-table-loading branch had added obs.to_memory() shims to those functions to handle Dataset2D-backed obs from anndata.experimental.read_lazy. Resolution: take upstream's structure. The new join-based code path does not do the pd.DataFrame(table.obs) conversion that originally motivated the shim, and _inplace_fix_subset_categorical_obs already handles Dataset2D obs, so the lazy-table fix continues to apply through that path. The shims in get_values and _inplace_fix_subset_categorical_obs are untouched.
Tomatokeftes
added a commit
to Tomatokeftes/spatialdata
that referenced
this pull request
Jul 30, 2026
Upstream scverse#1131 rewrote the join helpers to call pandas methods on table.obs directly (reset_index, groupby). A table read with anndata.experimental.read_lazy exposes obs as an xarray Dataset2D, which implements neither, so join_spatialelement_table, bounding_box_query and get_values all raised AttributeError on lazy tables. Route those five call sites through _obs_as_dataframe(), which materializes obs only when it is not already a DataFrame. obs is the small axis, so X stays lazy. get_values additionally needs X computed before it reaches pd.DataFrame(), which cannot consume a dask array; that is a selection of the requested columns only. Adds a regression test covering all five join modes, bounding_box_query and get_values against a lazy table, asserting the results equal the eager read and that X is still a dask array afterwards. The test fails without this change.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR refactors
relational_query.py, addressing some TODOs in the code that suggested replacing old query functions with the join operations. Also, this PR implements suggestions from an old PR from @giovp #701.